fix(ux): Show full channel name on hover#38320
fix(ux): Show full channel name on hover#38320dionisio-bot[bot] merged 4 commits intoRocketChat:developfrom
Conversation
|
|
Looks like this PR is ready to merge! 🎉 |
|
|
WalkthroughThree sidebar item component variants (Condensed, Extended, and Medium) are updated to explicitly pass the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Tested the PR locally it's showing the name on hover in sidebar, but try using the default PR template / be more descriptive or add a video, might increase the chances of it being reviewed and make it easy for maintainers as well. |
|
Summarise pr @coderabbitai |
|
Tip For best results, initiate chat on the files or code changes.
SummaryProblem: Long channel names in the sidebar were truncated without any way to see the full name on hover, requiring users to click into channels to identify them (issue Solution: Added the Changes Made:
Verification: Successfully tested locally by This is a small UX improvement that reduces unnecessary clicks and improves channel discoverability, especially for descriptive/long channel names. |
dougfabris
changed the title
dougfabris
left a comment
dougfabris
left a comment
There was a problem hiding this comment.
I don't think we need to check the type of the param neither the test you added for this small change
dougfabris
added
the
valid
tushargr0ver
force-pushed
the
fix-38316-sidebar-tooltip
branch
from
996bccc to
46c7963
Compare
|
Agreed, cleaned it up @dougfabris |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/meteor/client/sidebar/Item/Condensed.tsx (1)
30-32:⚠️ Potential issue | 🟠 MajorSame
ReactNode→titletype-safety issue asExtended.tsx
titleisReactNode(Line 6) but is passed without a string guard toSidebarV2ItemTitle title. Apply the same fix described forExtended.tsx.🛡️ Proposed fix
- <SidebarV2ItemTitle unread={unread} title={title}> + <SidebarV2ItemTitle unread={unread} title={typeof title === 'string' ? title : undefined}> {title} </SidebarV2ItemTitle>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/client/sidebar/Item/Condensed.tsx` around lines 30 - 32, The Condensed component is passing a ReactNode prop title directly into SidebarV2ItemTitle.title which expects a string; add a type guard like in Extended.tsx so you only pass a string (e.g., titleProp = typeof title === "string" ? title : undefined) to SidebarV2ItemTitle (keep the children render {title} unchanged). Update the usage inside Condensed.tsx to pass the guarded titleProp to the title attribute of SidebarV2ItemTitle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/meteor/client/sidebar/Item/Condensed.tsx`:
- Around line 30-32: The Condensed component is passing a ReactNode prop title
directly into SidebarV2ItemTitle.title which expects a string; add a type guard
like in Extended.tsx so you only pass a string (e.g., titleProp = typeof title
=== "string" ? title : undefined) to SidebarV2ItemTitle (keep the children
render {title} unchanged). Update the usage inside Condensed.tsx to pass the
guarded titleProp to the title attribute of SidebarV2ItemTitle.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/short-names-visible.mdapps/meteor/client/sidebar/Item/Condensed.tsxapps/meteor/client/sidebar/Item/Extended.tsxapps/meteor/client/sidebar/Item/Medium.tsx
✅ Files skipped from review due to trivial changes (1)
- .changeset/short-names-visible.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/sidebar/Item/Medium.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/sidebar/Item/Extended.tsxapps/meteor/client/sidebar/Item/Condensed.tsx
🔇 Additional comments (1)
apps/meteor/client/sidebar/Item/Extended.tsx (1)
63-65:title={title}passesReactNodeto a prop expectingstring— add type guard for defensive programming
titleis typed asReactNodeinExtendedPropsbut is passed directly to thetitleprop ofSidebarV2ItemTitle. The HTMLtitleattribute expects astring; passing non-string values will render as[object Object]in the tooltip. This pattern exists in Extended.tsx, Medium.tsx, and Condensed.tsx.While the exact type signature of
SidebarV2ItemTitle.titlecannot be verified in this environment (it's from the external@rocket.chat/fuselagepackage), the defensive fix aligns with best practices for handlingReactNode→stringconversions:- <SidebarV2ItemTitle unread={unread} title={title}> + <SidebarV2ItemTitle unread={unread} title={typeof title === 'string' ? title : undefined}> {title} </SidebarV2ItemTitle>Apply this fix to Extended.tsx (line 63), Medium.tsx, and Condensed.tsx.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38320 +/- ##
===========================================
- Coverage 70.73% 70.68% -0.06%
===========================================
Files 3195 3195
Lines 113105 113105
Branches 20510 20535 +25
===========================================
- Hits 80010 79947 -63
- Misses 31049 31099 +50
- Partials 2046 2059 +13
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
dionisio-bot
bot
removed
the
stat: ready to merge
coderabbitai
bot
added
the
type: feature
|
/jira COMM |
dionisio-bot
bot
added
the
stat: ready to merge
dbe69af
5 participants
Proposed changes (including videos or screenshots)
Currently, long channel names in the sidebar are truncated with an ellipsis. This PR adds a tooltip (title attribute) to the sidebar items so that the full channel name is visible on hover.
Video Evidence
sidebar-channel-hover-tooltip.webm
Issue(s)
Closes #38316
Steps to test or reproduce
Further comments
This is a small UX improvement to help users identify channels with similar prefixes.
Summary by CodeRabbit
COMM-144
Task: COMM-140